Removes arbitrary limits from eqValues & eqArray #303
Conversation
|
My guess is that this prevents the entire test application from crashing, because if brightscript enforces their limits, the current test crashing could bring down the app. If this is the justification for this feature existing, then I'm in favor of keeping the check. Perhaps rather than removing the checks, we should make the function return @georgejecook can you chime in here with why you added this functionality in the first place? |
|
Yeah, sounds reasonable regarding not wanting to crash the tests but Rooibos is enforcing limitations which don't exist in BrightScript i.e the implementation code might be perfectly fine but Rooibos would fail it because of an arbitrary limit, assuming we If the behaviour is desired by default, perhaps we could introduce a new config flag to ignore it? |
|
We do this because in situations where you have a circular graph, it crashes. this was a crude workaround I added years ago, that worked fine for the use case I had (in maestro, classes have a reference to top, and when debugging, top dumps the class fields, which include top - though many other opportunities exist to get this to happen). I think it's definitely worthy of revisiting, and apoligize for the headaches this must have caused you and others.. I wish I used ticktick back then ;) |
We're seeing
REACHED MAX ITERATIONS DOING COMPARISONwarnings in our unit tests which can potentially result in false positives sincetrueis always returned once the call count is reached.I was in the process of making this max iteration count configurable but what would be a sensible default? Should we enforce a max call count outside the BrightScript engine?
As it stands, these warnings are logged to console and are easily missed, especially when the test suite is of a significant size.
These changes are a proposal to remove the hardcoded call count in favour of letting BrightScript enforce stack limits.